Skip to content

[Issue #798] Transforms PoC: TypeScript#825

Draft
SnowboardTechie wants to merge 7 commits into
HOLD-transformsfrom
bryan/issue-798-transforms-poc-typescript
Draft

[Issue #798] Transforms PoC: TypeScript#825
SnowboardTechie wants to merge 7 commits into
HOLD-transformsfrom
bryan/issue-798-transforms-poc-typescript

Conversation

@SnowboardTechie
Copy link
Copy Markdown
Collaborator

@SnowboardTechie SnowboardTechie commented May 12, 2026

Summary

TypeScript proof-of-concept for the plugin transformation interface under @common-grants/sdk/extensions, mirroring the Python PoC (#810) so the ADR-0022 / ADR-0017 contract is validated in both SDKs. Ships buildTransforms(), TransformResult<T>, PluginError, the mapping runtime, a definePlugin() extension for meta + transformSchemas, and a runnable round-trip example.

Changes proposed

New public surface under @common-grants/sdk/extensions:

  • buildTransforms() — compiles a pair of ADR-0017 mapping objects into typed (toCommon, fromCommon) callables with call-time structural validation. Optional commonModel Zod schema turns parse failures into PluginError[] instead of thrown exceptions.
  • TransformResult<T> — unconditional { result, errors } return shape (ADR-0022 Decision ADR - Specification framework #7).
  • PluginError — structured error carrying path, handler, sourceValue, cause (ADR-0022 Decision ADR - Website hosting #9). sourceValue and cause are plain enumerable fields; per Decision ADR - Website hosting #9, the SDK does not redact by default. Adopters apply a redacted projection ({ name, message, path, handler }, documented in the README) before logging. message is also data-bearing on the Zod-validation path because Zod's default error map embeds rejected runtime values; full-message sanitization is tracked under [Py SDK] Implement plugin-output + mapping-definition validation #744.
  • transformFromMapping(), getFromPath(), DEFAULT_HANDLERS — mapping runtime, six built-in handlers (const, field, match, switch alias, numberToString, stringToNumber).
  • definePlugin() accepts optional meta: PluginMeta and transformSchemas: Partial<Record<ExtensibleSchemaName, ObjectSchemasInput>>. Existing callers passing only extensions are unaffected.
  • New supporting types: Handler, ObjectSchemasInput, ObjectSchemas, PluginMeta, PluginCapability, ObjectMappings, PluginExtensionsObjectConfig, PluginExtensions, TransformSchemasInput.

Behavior changes from a post-PoC review iteration (each surfaces a typo'd mapping at the earliest possible point rather than silently producing wrong output):

  • switchOnValue (the match / switch handler) throws on a non-object spec — surfaces as PluginError(handler: "match") via the walker. Catches mappings like { match: "literal-where-spec-belongs" } that previously fell through to silent undefined. Python crashes generically via AttributeError on the same input; TS just fails more helpfully.
  • stringToNumber rejects empty / whitespace-only strings and integer-shaped strings outside Number.MAX_SAFE_INTEGER. Number("") silently coerces to 0 in JavaScript; large integer-shaped strings silently lose precision ("9999999999999999999"1e19). Both surface as PluginError(handler: "stringToNumber") instead.

Security guards (mapping JSON may be reconstituted from untrusted sources via mergeExtensions(), per ADR-0022 Decision #8):

  • __proto__ rejected as an output field name at buildTransforms() call time (validateMapping()) and at walk time (transformFromMapping()).
  • getFromPath() traverses with Object.prototype.hasOwnProperty.call rather than in, so attacker-controlled field paths cannot resolve to prototype-chain properties.
  • Custom handler names that collide with DEFAULT_HANDLERS or shadow Object.prototype keys (constructor, toString, __proto__, etc.) rejected at buildTransforms() call time.
  • Recursion depth capped at a shared DEFAULT_MAX_TRANSFORM_DEPTH = 500 for both the build-time validateMapping() walk and the runtime transformFromMapping() walk, so adversarial mapping JSON cannot pass build-time validation only to blow the stack at runtime.
  • PluginError PII contract per ADR-0022 Decision ADR - Website hosting #9 — SDK does not redact by default. sourceValue and cause are enumerable and flow through JSON.stringify(err); adopters use the recommended { name, message, path, handler } projection before logging. Tests assert PII flows by default and that the projection contains it.
  • stringToNumber's thrown message does not embed the source value (it would otherwise flow into PluginError.message, which is harder to redact than a separate field).
  • Handler JSDoc documents two contracts custom-handler authors must respect: don't return objects with a __proto__ key (the walker treats handler return values as opaque), and don't throw Errors with PII in .message (it flows verbatim into PluginError.message).

Out of scope (deferred to full SDK):

Files:

  • New: src/extensions/transforms.ts, src/extensions/transformation.ts, examples/transforms.ts, __tests__/extensions/transforms.spec.ts, __tests__/extensions/transformation.spec.ts, .changeset/transforms-poc-typescript.md
  • Modified: src/extensions/types.ts (10 new transform types), src/extensions/define-plugin.ts (meta + transformSchemas), src/extensions/index.ts (exports a superset of Python's extensions/__init__.py surface — TS-only types listed in the barrel comment), src/extensions/README.md (new "Plugin transformations" section + API reference table), __tests__/extensions/define-plugin.spec.ts (five new tests), package.json (example:transforms script; ci now chains the example as a smoke step).

Context for reviewers

How verified:

  • pnpm run checks — eslint, prettier, tsc --noEmit clean (0 warnings).
  • pnpm run test — 448 tests across 24 suites. PoC contributes 39 handler-runtime tests (transformation.spec.ts) including getFromPath prototype-chain safety; 26 buildTransforms tests (transforms.spec.ts) including handler-shadow rejection for __proto__ / toString / default-name collisions beyond field, build-time __proto__ output-field rejection at root and nested positions, fromCommonMapping depth-cap symmetry, sibling-key parity pinning, and PII flow-through + adopter-supplied redacted-projection coverage; and 5 definePlugin tests for meta + transformSchemas.
  • pnpm run ci — full chain (checks → build → test → example:transforms). The example:transforms smoke step round-trips a synthetic grants.gov record through toCommon → Zod-validated CommonGrants OpportunityfromCommon with custom join / split handlers and verified opportunity_number recovery, so example breakage fails CI instead of going unnoticed.

Conform-before-extend. ADR-0022's TypeScript code blocks and the Python PoC at 799-transform-poc-fetch were treated as the spec, transliterated snake_case → camelCase, swapped Pydantic → Zod, and reused existing TS SDK type patterns (const generics, mapped types). Intentional divergences from the ADR's TS shape documented in code comments:

  • DefinePluginOptions.transformSchemas (not schemas) mirrors Python's transform_schemas workaround for the collision with the existing Plugin.schemas field. Full SDK target: resolve in [TSX SDK] Extend definePlugin() to accept schemas with toCommon/fromCommon #756.
  • BuildTransformsOptions.commonModel: z.ZodType<TCommon, z.ZodTypeDef, any> uses any in the contravariant input position to accept schemas with input/output asymmetry (e.g. .transform() producing Date from string). The ADR's ZodType<TCommon> would reject the SDK's own OpportunityBaseSchema.
  • buildTransforms() takes an options object { toCommonMapping, fromCommonMapping, handlers?, commonModel? } rather than the ADR's positional (toCommonMapping, fromCommonMapping, handlers?). Matches definePlugin()'s shape and leaves room for commonModel and future flags. ADR cosmetic update tracked alongside the full SDK work.

Cross-SDK behavior notes vs. Python PoC #810

The two SDKs converge on behavior for all mapping shapes that produce valid output. Where they differ on failure modes, the differences fall into two categories:

Same fail-loud direction, different error quality. Both SDKs reject the input; TS produces a more descriptive error message:

  • switchOnValue non-object spec: Python AttributeError on .get(); TS throws Error("match/switch handler: spec must be an object") wrapped as PluginError(handler: "match").
  • stringToNumber empty / whitespace string: Python int("") raises ValueError; TS throws a descriptive message via PluginError(handler: "stringToNumber").

JS-specific concerns with no Python analogue. Defenses against attack surfaces that don't exist in Python:

  • __proto__ build / walk / scrub defenses (prototype pollution is JS-only).
  • Object.prototype-shadow handler-name rejection.
  • getFromPath Object.prototype.hasOwnProperty.call over in.
  • stringToNumber unsafe-integer rejection (Python ints are arbitrary precision, so no analogous precision-loss concern).

Open ADR question worth surfacing for both SDKs: ADR-0022 Decision #9 says "the SDK does not redact by default." This PR's earlier iteration added non-enumerable + toJSON() hardening to redact sourceValue / cause on the JSON.stringify path; that hardening was removed to align with the ADR text. Worth a separate conversation on whether to amend Decision #9 to allow (or require) default JSON redaction in both SDKs.

Why HOLD-transforms. Issue #798 sits in the transforms feature bucket (#656, #745, #756, #757, #768, #798, #799, #813) per the branching strategy doc. The bucket batches to main at the C2 (July 21) checkpoint. PR #810 already targets HOLD-transforms for the Python PoC.

Additional information

Example output (from pnpm example:transforms):

=== toCommon (native → CommonGrants) ===
{
  "id": "a1b2c3d4-e5f6-7890-abcd-ef1234567890",
  "title": "Research into conservation techniques",
  "status": { "value": "open" },
  "customFields": {
    "legacyId":      { "name": "legacyId",      "fieldType": "integer", "value": 12345 },
    "agencyName":    { "name": "agencyName",    "fieldType": "string",  "value": "Department of Examples" },
    "applicantTypes": { "name": "applicantTypes", "fieldType": "array",  "value": ["state_governments"] },
    "compositeLabel": { "name": "compositeLabel", "fieldType": "string", "value": "ABC-123-XYZ-001 — Research into conservation techniques" }
  },
  ...
}

=== fromCommon (CommonGrants → native) ===
{
  "data": {
    "opportunity_uuid": "a1b2c3d4-e5f6-7890-abcd-ef1234567890",
    "opportunity_title": "Research into conservation techniques",
    "opportunity_number": "ABC-123-XYZ-001",  // recovered from compositeLabel via split handler
    ...
  }
}

✓ round-trip verified for fields covered by both mappings

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file sdk Issue or PR related to our SDKs typescript Issue or PR related to TypeScript tooling ts-sdk Related to TypeScript SDK labels May 12, 2026
@SnowboardTechie SnowboardTechie added the enhancement New feature or request label May 12, 2026
@SnowboardTechie SnowboardTechie force-pushed the bryan/issue-798-transforms-poc-typescript branch 2 times, most recently from 1754b34 to b408bef Compare May 14, 2026 16:40
SnowboardTechie added a commit that referenced this pull request May 14, 2026
Two foot-guns flagged in code review on PR #825:

1. Integer-shaped strings beyond Number.MAX_SAFE_INTEGER silently lost
   precision via Number(s) (e.g. "9999999999999999999" → 1e19). Plugin
   authors round-tripping 64-bit IDs through stringToNumber would have
   seen silent corruption. Guard the integer branch with
   Number.isSafeInteger and throw on overflow; callers needing arbitrary
   precision should declare the field as a string or write a custom
   handler returning BigInt.

2. Empty and whitespace-only strings coerced to 0 via Number("") /
   Number("  "), turning an implicit-absent CSV cell into a real zero on
   the transformed side. Explicit s === "" check after .trim() throws on
   both cases.

Docstring rewritten to document both divergences from Python's int()
alongside the existing "42.0" decimal-fallback note. Six new tests pin
the boundaries (MAX/MIN_SAFE_INTEGER accepted, beyond-safe rejected on
both signs, empty and whitespace-only rejected).

pnpm run checks clean; pnpm test 443/443; example:transforms round-trip
still verifies (legacy ID 12345 well within safe-integer range).
@SnowboardTechie
Copy link
Copy Markdown
Collaborator Author

HOLD-transforms synced from main on 2026-05-20: 12 commits — 7 Dependabot / catalog dep bumps (#829, #826, #822, #821, #819, #817, #808), 3 CI / docs chores (#834, #820, #816), 2 release version bumps [skip ci]. Merge was clean (no conflicts). You may want to rebase or merge HOLD-transforms back into your branch.

@SnowboardTechie SnowboardTechie force-pushed the bryan/issue-798-transforms-poc-typescript branch from 2a4f4c6 to 1fdd6e7 Compare May 20, 2026 17:08
SnowboardTechie added a commit that referenced this pull request May 20, 2026
Two foot-guns flagged in code review on PR #825:

1. Integer-shaped strings beyond Number.MAX_SAFE_INTEGER silently lost
   precision via Number(s) (e.g. "9999999999999999999" → 1e19). Plugin
   authors round-tripping 64-bit IDs through stringToNumber would have
   seen silent corruption. Guard the integer branch with
   Number.isSafeInteger and throw on overflow; callers needing arbitrary
   precision should declare the field as a string or write a custom
   handler returning BigInt.

2. Empty and whitespace-only strings coerced to 0 via Number("") /
   Number("  "), turning an implicit-absent CSV cell into a real zero on
   the transformed side. Explicit s === "" check after .trim() throws on
   both cases.

Docstring rewritten to document both divergences from Python's int()
alongside the existing "42.0" decimal-fallback note. Six new tests pin
the boundaries (MAX/MIN_SAFE_INTEGER accepted, beyond-safe rejected on
both signs, empty and whitespace-only rejected).

pnpm run checks clean; pnpm test 443/443; example:transforms round-trip
still verifies (legacy ID 12345 well within safe-integer range).
@SnowboardTechie SnowboardTechie force-pushed the bryan/issue-798-transforms-poc-typescript branch from 1fdd6e7 to f97a3ba Compare May 21, 2026 16:29
SnowboardTechie added a commit that referenced this pull request May 21, 2026
Two foot-guns flagged in code review on PR #825:

1. Integer-shaped strings beyond Number.MAX_SAFE_INTEGER silently lost
   precision via Number(s) (e.g. "9999999999999999999" → 1e19). Plugin
   authors round-tripping 64-bit IDs through stringToNumber would have
   seen silent corruption. Guard the integer branch with
   Number.isSafeInteger and throw on overflow; callers needing arbitrary
   precision should declare the field as a string or write a custom
   handler returning BigInt.

2. Empty and whitespace-only strings coerced to 0 via Number("") /
   Number("  "), turning an implicit-absent CSV cell into a real zero on
   the transformed side. Explicit s === "" check after .trim() throws on
   both cases.

Docstring rewritten to document both divergences from Python's int()
alongside the existing "42.0" decimal-fallback note. Six new tests pin
the boundaries (MAX/MIN_SAFE_INTEGER accepted, beyond-safe rejected on
both signs, empty and whitespace-only rejected).

pnpm run checks clean; pnpm test 443/443; example:transforms round-trip
still verifies (legacy ID 12345 well within safe-integer range).
@github-actions github-actions Bot added cli Issue or PR related to the @common-grants/cli library website Issues related to the website core Issues related to @common-grants/core library python Issue or PR related to Python tooling py-sdk Related to Python SDK labels May 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🚀 Website Preview Deployed!

Preview your changes at: https://cg-pr-825.billy-daly.workers.dev

This preview will be automatically deleted when the PR is closed.

@SnowboardTechie SnowboardTechie force-pushed the bryan/issue-798-transforms-poc-typescript branch from f97a3ba to 1897cb1 Compare May 21, 2026 17:01
SnowboardTechie added a commit that referenced this pull request May 21, 2026
Two foot-guns flagged in code review on PR #825:

1. Integer-shaped strings beyond Number.MAX_SAFE_INTEGER silently lost
   precision via Number(s) (e.g. "9999999999999999999" → 1e19). Plugin
   authors round-tripping 64-bit IDs through stringToNumber would have
   seen silent corruption. Guard the integer branch with
   Number.isSafeInteger and throw on overflow; callers needing arbitrary
   precision should declare the field as a string or write a custom
   handler returning BigInt.

2. Empty and whitespace-only strings coerced to 0 via Number("") /
   Number("  "), turning an implicit-absent CSV cell into a real zero on
   the transformed side. Explicit s === "" check after .trim() throws on
   both cases.

Docstring rewritten to document both divergences from Python's int()
alongside the existing "42.0" decimal-fallback note. Six new tests pin
the boundaries (MAX/MIN_SAFE_INTEGER accepted, beyond-safe rejected on
both signs, empty and whitespace-only rejected).

pnpm run checks clean; pnpm test 443/443; example:transforms round-trip
still verifies (legacy ID 12345 well within safe-integer range).
@SnowboardTechie
Copy link
Copy Markdown
Collaborator Author

Synced mainHOLD-transforms on 2026-05-21 (merge commit 211ecec). 9 commits landed:

Notable for this PR: the lockfile-side effect of #842 bumps brace-expansion 5.0.5 → 5.0.6, which resolves GHSA-jxxr-4gwj-5jf2 — the advisory the Audit dependencies CI step was flagging. Re-run CI on this PR to refresh against the updated base.

Port the Python transforms PoC (PR #810, branch 799-transform-poc-fetch)
to @common-grants/sdk so the ADR-0022 / ADR-0017 contract is validated
in both SDKs before either is locked in for full implementation.

Public additions under @common-grants/sdk/extensions:

- buildTransforms() — compile a pair of ADR-0017 mapping objects into typed
  (toCommon, fromCommon) callables with call-time structural validation.
  Optional commonModel Zod schema turns parse failures into PluginError[]
  rather than thrown exceptions.
- TransformResult<T> — unconditional { result, errors } return shape
  (ADR-0022 Decision #7).
- PluginError — structured error class with path / handler / sourceValue /
  cause (ADR-0022 Decision #9). sourceValue and cause are stored
  non-enumerable; toJSON() omits them so JSON.stringify(err) is PII-safe.
  console.log(err) / util.inspect(err) still render [cause] via Node's
  default Error inspection — README PII callout calls this out and
  recommends a redacted projection. Zod-validation message is also
  data-bearing because Zod's default error map embeds rejected values;
  full-message sanitization tracked under #744.
- transformFromMapping(), getFromPath(), DEFAULT_HANDLERS — mapping
  runtime; six built-in handlers (const, field, match, switch alias,
  numberToString, stringToNumber).
- definePlugin() accepts optional meta and transformSchemas. Existing
  callers passing only `extensions` are unaffected.

Security hardening (mapping JSON may be reconstituted from untrusted
sources via mergeExtensions(), so the runtime must fail loud on hostile
shapes):

- buildTransforms() rejects custom handler names that collide with the
  default registry or shadow Object.prototype keys (constructor, toString,
  __proto__, etc.) at call time.
- validateMapping() rejects `__proto__` as an output field name at build
  time; transformFromMapping() rejects it again at walk time so the JSON
  attack vector (own-enumerable __proto__ key from JSON.parse) fails fast
  in both places. Shared DEFAULT_MAX_TRANSFORM_DEPTH = 500 across both
  walkers so adversarial mapping JSON can't pass build-time validation
  only to blow the stack at runtime.
- transformFromMapping() scrubs top-level own `__proto__` from
  plain-object handler returns (defense-in-depth: const / field / match
  can return JSON.parse-loaded objects with own __proto__ keys, and a
  downstream for-in deep-merge of the result would otherwise pollute
  Object.prototype).
- getFromPath() uses Object.prototype.hasOwnProperty.call rather than
  `in` so attacker-controlled field paths cannot resolve to
  prototype-chain properties.
- stringToNumber's error message does not embed the source value (would
  flow into PluginError.message and bypass the sourceValue PII guard).
- Handler JSDoc documents two contracts custom-handler authors must
  respect: don't return objects with a `__proto__` key (the walker
  treats handler return values as opaque beyond the top-level scrub),
  and don't throw Errors with PII in .message (it flows verbatim into
  PluginError.message, which is rendered by Node Error inspection).

Cross-SDK parity:

- switchOnValue mirrors Python's `lookup.get(val, default)` exactly:
  only string source values are candidate keys; numeric / boolean / object
  source values short-circuit to default. Pinned by parity tests.
- ClientConfig parity export added (Python `__init__.py` exports it as
  `dict[str, Any]`).
- Six placeholder type exports (ObjectSchemas, ObjectMappings,
  PluginExtensions, PluginExtensionsObjectConfig, PluginCapability,
  PluginMeta) define the ADR-0022 contract surface and parallel the
  Python PoC's extensions/__init__.py exports.

Documented divergences from ADR-0022's TS code blocks (justified in
findings.md):

- DefinePluginOptions.transformSchemas (not `schemas`) avoids collision
  with the existing Plugin.schemas field. Resolution deferred to #756.
- BuildTransformsOptions.commonModel uses
  `z.ZodType<TCommon, z.ZodTypeDef, any>` to admit schemas with
  input/output asymmetry (e.g. .transform() producing Date from string).
  ZodType<TCommon> would reject the SDK's own OpportunityBaseSchema.

Out of scope (matches Python PoC; deferred to full SDK):

- Auto-generation of transforms from declarative
  extensions.schemas[obj].mappings inside definePlugin() (Decision #6 TODO).
- Always-on commonModel validation inside definePlugin() — opt-in at
  buildTransforms() for now (Decision #7 TODO).
- Sanitizing Zod's default error map output before it lands in
  PluginError.message — tracked in #744.
- Defensive output-key rejection of __defineGetter__ / __defineSetter__ /
  constructor / toString — tracked in #744.
- Recursive (nested) sanitization of __proto__ in handler return values —
  tracked in #744 alongside other Decision #8 hardening.

Includes:
- examples/transforms.ts round-trip (`pnpm example:transforms`)
- README "Plugin transformations" section + API reference table
- 14 define-plugin specs, 35 transformation-handler specs,
  19 buildTransforms specs (439 tests across 24 suites, all passing)
- Minor changeset bump for @common-grants/sdk

Targets HOLD-transforms per the SDK Plugin Enhancements branching strategy.
Two foot-guns flagged in code review on PR #825:

1. Integer-shaped strings beyond Number.MAX_SAFE_INTEGER silently lost
   precision via Number(s) (e.g. "9999999999999999999" → 1e19). Plugin
   authors round-tripping 64-bit IDs through stringToNumber would have
   seen silent corruption. Guard the integer branch with
   Number.isSafeInteger and throw on overflow; callers needing arbitrary
   precision should declare the field as a string or write a custom
   handler returning BigInt.

2. Empty and whitespace-only strings coerced to 0 via Number("") /
   Number("  "), turning an implicit-absent CSV cell into a real zero on
   the transformed side. Explicit s === "" check after .trim() throws on
   both cases.

Docstring rewritten to document both divergences from Python's int()
alongside the existing "42.0" decimal-fallback note. Six new tests pin
the boundaries (MAX/MIN_SAFE_INTEGER accepted, beyond-safe rejected on
both signs, empty and whitespace-only rejected).

pnpm run checks clean; pnpm test 443/443; example:transforms round-trip
still verifies (legacy ID 12345 well within safe-integer range).
The runtime walker is first-key-wins, so `{ field: "x", const: "y" }`
would silently drop `const`. Almost always an author bug. Match Python
PoC `_validate_mapping` and fail loud at buildTransforms() call time.
The low-level transformFromMapping walker keeps lenient behavior so
programmatic users composing partial mappings aren't forced into the
strict shape.
Two behavior changes:

- PluginError: drop non-enumerable + toJSON() hardening to match
  ADR-0022 Decision #9 ("the SDK does not redact by default").
  README + JSDoc document the adopter-supplied redacted projection
  as the supported PII-safety path. Existing PII test inverted.
- switchOnValue: throw on non-object spec instead of silent undefined.
  Walker wraps as HandlerError → PluginError(handler: "match").

JSDoc / comment fixes: reframe "Mirrors Python PoC" claims that were
factually wrong (Python's _validate_mapping doesn't reject siblings;
Python has no __proto__ defenses; lookup.get accepts non-string val)
as honest "TS-only hardening; cf. #810 for parallel Python proposal."
Complete @throws lists on buildTransforms and transformFromMapping.
Document stringToNumber null/undefined return. Correct misleading
"re-exported" wording on DEFAULT_MAX_TRANSFORM_DEPTH. Correct
"trimmed to match Python's __init__.py" comment on the barrel
(TS exports a superset).

Test gap coverage (ADR-0022 Decision #8): getFromPath prototype-chain
safety; handler-shadow rejection for __proto__/toString/default-name
collisions beyond \`field\`; build-time __proto__ rejection in
validateMapping; fromCommonMapping depth-cap.

README + CI: add missing ClientConfig and TransformFromMappingOptions
rows to the API table; chain example:transforms into pnpm run ci as
a smoke step.

Simplify: extract deepMapping(levels) helper; PII test suite uses
beforeEach so both tests assert on the same PluginError instance.

Verification: pnpm run ci passes end-to-end (445 → 449 tests).

Cross-SDK follow-ups (file against #810 / amend ADR-0022 separately):
TS-only sibling-key rejection, __proto__ defenses, switchOnValue
non-object throw, stringToNumber strictness, handler-shadow rejection;
and the ADR-0022 Decision #9 default-redaction question itself.
Python's `_validate_mapping` accepts handler-dispatch nodes with sibling
keys; the TS `validateMapping` was rejecting them at build time, creating
the only genuine "TS adds mandatory validation Python doesn't" divergence
in this PR. The first-key-wins walker behavior already matches Python, so
both SDKs now share the same foot-gun: `{ field: "x", const: "fallback" }`
silently drops `const` in both.

Replaces the two rejection tests with one parity-pinning test
(`accepts (does not reject) sibling keys ... cross-SDK parity`) that
asserts `.not.toThrow()` — locks the decision so a future regression
re-introducing build-time rejection (and re-creating the divergence)
fails loudly.

Updates `validateMapping` JSDoc to call out the parity decision and
`transformFromMapping` JSDoc to drop the (now false) reference to a
stricter build-time check.

If we later want both SDKs to fail loud on this shape, the cleanest path
is to add the check to Python first and then re-introduce TS — keeps
divergence at zero.

Verification: pnpm run ci passes end-to-end (449 → 448 tests; -2 removed
rejection tests, +1 added parity-pinning test).
@SnowboardTechie SnowboardTechie force-pushed the bryan/issue-798-transforms-poc-typescript branch from 1897cb1 to 3e557b9 Compare May 21, 2026 18:18
Three findings from a /review pass over the PoC. All three tighten
existing contracts; none introduce new shapes.

- switchOnValue: reject arrays as spec. Arrays pass `typeof === "object"`
  and non-null but lack the structural shape — a mapping like
  `{ match: ["posted", "archived"] }` would otherwise silently resolve
  to s.field/s.case/s.default all-undefined and return undefined.
  Same fail-loud direction as the existing non-object guards.

- __proto__ scrub: shallow-clone on the pollution path rather than
  `delete`-in-place. `fieldValue` returns references plucked from caller
  input via `getFromPath`, so an in-place delete would silently mutate
  the caller's data — surprising for plugin authors caching parsed
  source records across `toCommon` calls (common in long-running adapter
  processes and multi-tenant deployments). Spread is the correct copy
  primitive here: CopyDataProperties -> CreateDataProperty bypasses the
  prototype setter. Object.assign would mutate the target's prototype
  chain instead — the source comment now warns against the regression.

- README error-handling snippet: log a named redacted projection rather
  than building the message via template-string interpolation. Makes
  the "what gets logged" surface a single audit point and matches the
  projection in the prose PII callout and in the redacted-projection
  test verbatim.

Verified: pnpm --filter @common-grants/sdk run ci passes (449 tests
across 24 suites, +1 source-preservation test; example:transforms
round-trip OK).
@SnowboardTechie SnowboardTechie removed python Issue or PR related to Python tooling py-sdk Related to Python SDK dependencies Pull requests that update a dependency file labels May 21, 2026
Mirror the Python PoC PR #838 commit a156d31 so plugin authors add a
single per-object entry under transformSchemas[Object] when introducing
a new object, rather than splitting customFields across a separate
top-level extensions dict.

- types.ts: add customFields?: Record<string, CustomFieldSpec> to
  ObjectSchemasInput; drop customFields from PluginExtensionsObjectConfig
  (now mappings-only, matching Python's PluginExtensionsSchema).
- define-plugin.ts: make extensions optional in DefinePluginOptions and
  Plugin; definePlugin() sources customFields from
  transformSchemas[name].customFields first, falling back to the legacy
  extensions[name] surface so existing customFields-only plugins keep
  working.
- examples/transforms.ts: consolidate to a single transformSchemas entry
  carrying customFields + toCommon + fromCommon together.
- tests: cover the consolidated path (customFields on transformSchemas[obj]),
  no-extensions-arg case, and transformSchemas-wins-over-extensions priority.

Open question — ADR-0022 as written places customFields inside
PluginExtensions.schemas[obj] specifically so declarations can be
combined across packages via mergeExtensions() (Decision driver line 23,
Decision #4). This commit follows Jeff's Python move out of that
serializable surface, which drops customFields from the cross-package
merge contract. Pending an ADR-0022 amendment formalizing the trade-off;
flagged inline on ObjectSchemasInput and PluginExtensionsObjectConfig.

Type inference for plugin.schemas[obj] still flows through the legacy
extensions parameter — the runtime applies customFields from either
source, but the compiled-schema type only narrows when customFields are
declared via extensions. Wiring the generic through transformSchemas is
a follow-up (parallel to the transformSchemas → schemas rename deferred
to #756).
@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Issue or PR related to the @common-grants/cli library core Issues related to @common-grants/core library dependencies Pull requests that update a dependency file enhancement New feature or request sdk Issue or PR related to our SDKs ts-sdk Related to TypeScript SDK typescript Issue or PR related to TypeScript tooling website Issues related to the website

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant